feat(resharding): Resharding state mapping integration #12269
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12269 +/- ##
=======================================
Coverage 71.24% 71.25%
=======================================
Files 838 838
Lines 169020 169074 +54
Branches 169020 169074 +54
=======================================
+ Hits 120424 120475 +51
+ Misses 43351 43350 -1
- Partials 5245 5249 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
staffik
force-pushed
the
resharding-mapping-integration
branch
from
9089366
to
103279d
Compare
shreyan-gupta
approved these changes
Oct 28, 2024
| /// created later. | ||
| pub fn process_memtrie_resharding_storage_update( | ||
| /// Trigger resharding if shard layout changes after the given block. | ||
| pub fn start_resharding( |
There was a problem hiding this comment.
Nice... thanks for renaming!
staffik
force-pushed
the
resharding-mapping-integration
branch
from
cbc01fa
to
2fc80bc
Compare
wacban
approved these changes
Oct 28, 2024
Comment on lines
88
to
101
| match resharding_event_type { | ||
| Some(ReshardingEventType::SplitShard(split_shard_event)) => { | ||
| self.split_shard( | ||
| chain_store_update, | ||
| block, | ||
| shard_uid, | ||
| tries, | ||
| split_shard_event, | ||
| next_shard_layout, | ||
| )?; | ||
| } | ||
| None => { | ||
| tracing::debug!(target: "resharding", ?resharding_event_type, "unsupported resharding event type, skipping"); | ||
| } |
| ) -> io::Result<()> { | ||
| let mut store_update = self.store.trie_store().store_update(); | ||
| let parent_shard_uid = split_shard_event.parent_shard; | ||
| // TODO(reshardingV3) No need to set the mapping for children shards that we won't track just after resharding? |
There was a problem hiding this comment.
Good call making that a todo. Let's come back to this question when we figure out the full picture for post resharding cleanup.
wacban
reviewed
Oct 29, 2024
Comment on lines
+121
to
137
| // Reshard the State column by setting ShardUId mapping from children to parent. | ||
| self.set_state_shard_uid_mapping(&split_shard_event)?; | ||
|
|
||
| // Create temporary children memtries by freezing parent memtrie and referencing it. | ||
| self.process_memtrie_resharding_storage_update( | ||
| chain_store_update, | ||
| block, | ||
| shard_uid, | ||
| vec![split_shard_event.left_child_shard, split_shard_event.right_child_shard], | ||
| tries, | ||
| split_shard_event.clone(), | ||
| )?; | ||
|
|
||
| // Trigger resharding of flat storage. | ||
| self.flat_storage_resharder.start_resharding( | ||
| ReshardingEventType::SplitShard(split_shard_event.clone()), | ||
| ReshardingEventType::SplitShard(split_shard_event), | ||
| &next_shard_layout, | ||
| )?; |
There was a problem hiding this comment.
I really like how this is shaping up - split_shard consists of splitting the State, FlatState and MemTrie.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tracking issue: #12050
Summary:
ReshardingManager::process_memtrie_resharding_storage_update()intostart_resharding()as it is an entry point to start resharding.set_state_shard_uid_mapping()and call it fromstart_resharding().resharding_v3.rstestloop.Test would fail with
MissingTrieValuefor children shards if disabledset_state_shard_uid_mapping.Perhaps
test_resharding_v3_baseshould be as minimal as possible and I should not test State mapping there. But for now I do not see much value in doing it differently.